Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add mutli arch build and manifest for the operator repo #11

Merged
merged 1 commit into from
Jun 3, 2024
Merged

Add mutli arch build and manifest for the operator repo #11

merged 1 commit into from
Jun 3, 2024
+153 −34

Conversation

msherif1234
Copy link
Contributor

MULTIARCH_TARGETS="ppc64le s390x" BPFMAN_AGENT_IMG=quay.io/bpfman/bpfman-agent:lastest BPFMAN_OPERATOR_IMG=quay.io/bpfman/bpfman-operator:latest make build-images

will result in

quay.io/bpfman/bpfman-agent      lastest-s390x      d0e6f5097fd1   About a minute ago   471MB
quay.io/bpfman/bpfman-agent      lastest-ppc64le    c9953f679b4a   4 minutes ago        585MB
quay.io/bpfman/bpfman-operator   latest-s390x       78079918de28   14 minutes ago       177MB
quay.io/bpfman/bpfman-operator   latest-ppc64le     7ec1a6abcb74   15 minutes ago       238MB

@codecov Codecov
Copy link

codecov bot commented May 31, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.38%. Comparing base (eb341c4) to head (c2f4b4b).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main      #11   +/-   ##
=======================================
  Coverage   26.38%   26.38%           
=======================================
  Files          75       75           
  Lines        5021     5021           
=======================================
  Hits         1325     1325           
  Misses       3532     3532           
  Partials      164      164
Flag Coverage Δ
unittests 26.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Billy99
Billy99 reviewed May 31, 2024
View reviewed changes
Copy link
Contributor

@Billy99 Billy99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just cosmetic changes.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@msherif1234 msherif1234 requested a review from Billy99 May 31, 2024 16:49
@msherif1234 msherif1234 force-pushed the mutli_arch branch 6 times, most recently from cb4c9de to 06a36ba Compare May 31, 2024 17:52
astoycos
astoycos reviewed May 31, 2024
View reviewed changes
Makefile Outdated
.PHONY: manifest-build
manifest-build: ## Build MULTIARCH_TARGETS manifest for bpfman-operator and bpfman-agent.
echo 'building manifest for $(BPFMAN_OPERATOR_IMG) and $(BPFMAN_AGENT_IMG)'
DOCKER_BUILDKIT=1 $(OCI_BIN) rmi ${BPFMAN_OPERATOR_IMG} -f
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens here if $(OCI_BIN) is podman? the DOCKER_BUILDKIT=1 won't have any effect in that scenario right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OCI_BIN is docker if u have docker installed

Copy link

@jpinsonneau jpinsonneau Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOCKER_BUILDKIT is indeed ignored using podman however it works fine here (tested locally)

In addition to that, since docker engine 23, buildkit is the default behavior so we could even get rid of it if we mention this as prereqs in the README

astoycos
astoycos reviewed May 31, 2024
View reviewed changes
Makefile Outdated
# build a single arch target provided as argument
define build_target
echo 'building $(1) for arch $(2)'; \
DOCKER_BUILDKIT=1 $(OCI_BIN) buildx build --load --build-arg TARGETPLATFORM=linux/$(2)\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again will this work if OCI_BIN is podman?

@msherif1234 msherif1234 force-pushed the mutli_arch branch 8 times, most recently from a438509 to 85a029e Compare May 31, 2024 19:56
@msherif1234 msherif1234 requested a review from astoycos May 31, 2024 20:13
@msherif1234 msherif1234 force-pushed the mutli_arch branch 8 times, most recently from 48ad0c6 to 6d7564a Compare June 1, 2024 02:06
@msherif1234 msherif1234 force-pushed the mutli_arch branch 3 times, most recently from 065706b to a3acdb0 Compare June 3, 2024 11:03
@astoycos
Copy link
Member

astoycos commented Jun 3, 2024

LGTM

@astoycos astoycos merged commit bbfd3dc into bpfman:main Jun 3, 2024
8 checks passed
msherif1234 added a commit to msherif1234/bpfman-operator that referenced this pull request Oct 10, 2024
@msherif1234
Merge pull request bpfman#11 from msherif1234/fix-catalog-4.18
56234b3 
Fix catalog 4.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpinsonneau jpinsonneau jpinsonneau left review comments

@astoycos astoycos astoycos approved these changes

@Billy99 Billy99 Awaiting requested review from Billy99

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@msherif1234 @astoycos @Billy99 @jpinsonneau